regarding the video elements: I'm refactoring the hierarchy like this:

// old hierarchy
bentovideo > pbe > ...
ampvideobase > bentovideo > ...
ampvideo > ampvideobase > ...
bentobrightcove > ampvideobase > ...
ampbrightcove > bentobrightcove > ...
^^ bento-video/brightcove extends amp video stuff which isn't ideal

// new hierarchy
bentovideo > pbe > ...
ampvideobase > pbe > ...
ampvideo > ampvideobase > ...
bentobrightcove > bentovideo > ...
ampbrightcove > ampvideobase > ...

Notes:
bentovideo: is a shared base class for all bento video components. It is also the class for . (its name is staying BaseElement so that the autogenerated web-component.js still works, but can be imported as BentoVideoBaseElement)

ampvideobase: is a shared base class for all amp video components (VideoBaseElement is changing to AmpVideoBaseElement)
bentovideo and ampvideo will share common video component utilities in extensions/amp-video/element.js

This pull request introduces 2 alerts when merging d122f29 into 58a1c29 - view on LGTM.com

new alerts:

• 1 for Useless assignment to local variable
• 1 for Variable not declared before use

This pull request introduces 2 alerts when merging f28fdbc into b6124cb - view on LGTM.com

new alerts:

• 1 for Useless assignment to local variable
• 1 for Variable not declared before use

This pull request introduces 2 alerts when merging b13499e into faf69c4 - view on LGTM.com

new alerts:

• 1 for Useless assignment to local variable
• 1 for Variable not declared before use

This pull request introduces 2 alerts when merging e78365b into bfd4f7f - view on LGTM.com

new alerts:

• 1 for Useless assignment to local variable
• 1 for Variable not declared before use

splitting out prelim commits here: #37181

Hey @westonruter, @ediamin! These files were changed:

extensions/amp-wordpress-embed/1.0/amp-wordpress-embed.js
extensions/amp-wordpress-embed/1.0/base-element.js
extensions/amp-wordpress-embed/1.0/element.js

Is there any way to split this into smaller PRs?

Is there any way to split this into smaller PRs?

I can split this into 1 PR for each commit (I tried to make sure each commit was isolated and related to 1 extension). Would help to get PRs in 1 extension at a time. But would create ~26 PRs, so i opted against it. Would you like me to do that?

closing in favor of: #37309

closed because the design of AMP components not inheriting from Bento components meant that we have to reimplement a lot of logic in both AMP and Bento classes. This could be a an easy source of bugs in the future if we implement the logic differently or they diverge accidentally. (this was discussed offline)